Skip to content

Conversation

@yurem
Copy link
Contributor

@yurem yurem commented Jan 6, 2026

##13062

Closes #13062,

Summary by CodeRabbit

  • New Features

    • Adds gRPC server (multiple modes), gRPC audit service and REST↔gRPC bridge with authorization enforcement.
    • Audit REST endpoints accept JSON and support single & bulk audit APIs; new gRPC-based audit API available.
    • New configuration options for gRPC behavior and a flag to disable external logger configuration; HTTP/2 cleartext proxy support for lock service.
  • Bug Fixes

    • Corrected severity field typo in audit/log schema.
  • Chores

    • Added protobuf/grpc build tooling, header/token helpers, access policies for bulk writes, and an end-to-end test script.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 6, 2026

📝 Walkthrough

Walkthrough

Adds end-to-end gRPC audit support and bridge: protobuf service, gRPC server/servlet adapter, authorization interceptor, header/IP utilities, model/schema enrichments, REST audit endpoints updated to typed payloads and bulk lists, Jetty/Apache http2c proxying, build/plugin updates, and refactors protection interfaces to accept bearerToken strings.

Changes

Cohort / File(s) Summary
Build & Maven
jans-bom/pom.xml, jans-lock/lock-server/service/pom.xml
Add gRPC BOM imports/properties, protobuf & os-maven pluginManagement entries; add gRPC and resteasy-grpc dependencies; switch servlet API to javax.servlet-api:4.0.1.
Protobuf
.../service/src/main/proto/grpc/audit.proto
New audit proto with Health/Log/Telemetry messages, bulk variants, AuditResponse, and AuditService RPCs (6 methods).
gRPC Server & Starter
.../grpc/server/GrpcServerStarter.java, .../ServiceInitializer.java
New GrpcServerStarter to build/start/stop gRPC server (plain/TLS) and ServiceInitializer injection/init call.
gRPC Servlet / Bridge
.../grpc/servlet/GrpcAuditServlet.java
New servlet bridge that conditionally initializes a Netty adapter in BRIDGE mode and wraps requests with a header/URI-normalizing request wrapper.
gRPC Audit Implementation
.../grpc/audit/GrpcAuditServiceImpl.java, GrpcAuditServiceProvider.java, GrpcToJavaMapper.java
gRPC service implementation delegating to REST backend, CDI provider to expose service instance, and mapper converting protobuf messages → Java models.
Authorization Interceptor & Filters
.../grpc/security/GrpcAuthorizationInterceptor.java, .../service/filter/AuthorizationProcessingFilter.java, .../cedarling/*
New gRPC interceptor mapping method→ResourceInfo and invoking protection backends; filters updated to extract bearer tokens (helper added).
Protection Interfaces & Services
jans-core/.../ProtectedApi.java, .../service/security/protect/BaseAuthorizationProtection.java, .../openid/OpenIdProtection.java, .../openid/OpenIdProtectionService.java, cedarling services
Add grpcMethodName to ProtectedApi; introduce BaseAuthorizationProtection and OpenIdProtection interfaces using processAuthorization(String, ResourceInfo); refactor OpenID/Cedarling services to accept bearerToken string.
Header / IP Utilities
jans-lock/.../util/HeaderUtils.java, .../util/ServerUtil.java
New HeaderUtils for Metadata auth lookup and bearer extraction; ServerUtil adds CLIENT_IP_CONTEXT_KEY and helpers to resolve client IP from gRPC metadata/ServerCall or Context.
Audit Models (Java)
jans-lock/lock-server/model/src/main/java/io/jans/lock/model/audit/*
Enrich HealthEntry, LogEntry, TelemetryEntry: add creationDate/eventTime/service/nodeName/status, rename/replace fields (engineStatus→Map), numeric fields → Long, add JSON/Schema annotations and equals/hashCode/toString.
Config Models
.../model/config/GrpcServerMode.java, .../grpc/GrpcConfiguration.java, .../AppConfiguration.java
New GrpcServerMode enum; new GrpcConfiguration class with serverMode, port, TLS fields; AppConfiguration adds grpcConfiguration and disableExternalLoggerConfiguration.
REST Audit Endpoints
.../ws/rs/audit/AuditRestWebService.java, AuditRestWebServiceImpl.java
Endpoints now consume JSON and accept typed entry objects (single) and List<> for bulk; implementation refactored to typed handlers, client IP helper, and bulk persist helpers; ProtectedApi annotations updated with grpcMethodName.
Apache/Jetty & Installers
jans-linux-setup/.../https_jans.conf.mako, jetty_app_configuration.json, installers/jans_auth.py, installers/jans_lock.py
Apache template updated for gRPC proxying (Protocols h2 + gRPC proxy locations), Jetty modules append http2c when jans-lock installed, and apache_lock_config() removed from JansLock installer.
OpenAPI / Docs
jans-config-api/plugins/docs/lock-plugin-swagger.yaml, fido2-plugin-swagger.yaml, jans-lock/lock-server.yaml
Add GrpcConfiguration schema and disableExternalLoggerConfiguration flag; extend Health/Log/Telemetry schemas with descriptions, enums, examples and new fields.
Schema Fixes
jans-linux-setup/jans_setup/schema/jans_schema.json
Fix typo severetyLevelseverityLevel; add jti to jansLogEntry may attributes.
Audit Action Types
jans-lock/lock-server/model/.../AuditActionType.java
Add new enum constant GRPC_AUTHZ_FILTER.
Policies
jans-lock/lock-server/service/src/main/policy/audit/*.json
Add policies for bulk write endpoints (health/log/telemetry) requiring specific OAuth scopes and POST path checks.
Tests / Scripts
.../test/manual/automatic_audit_enpoints_tests.sh
New manual/integration script exercising REST↔gRPC bridge and authorization with curl + grpcurl.
Misc / License Headers & Minor Fixes
many jans-lock/... Java files
Multiple license/header text updates, small header/import adjustments, and other non-functional formatting changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~70 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • iromli
  • yuriyz
  • devrimyatar
  • pujavs
  • yuriyzz
🚥 Pre-merge checks | ✅ 1 | ❌ 4
❌ Failed checks (3 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. It only contains a single line 'Closes #13062,' without any implementation details, testing information, or documentation changes. Add implementation details explaining the gRPC audit endpoints, testing information, and confirm documentation status by filling out the template sections provided.
Out of Scope Changes check ⚠️ Warning The PR includes changes beyond gRPC audit endpoints, such as license header updates across multiple files and unrelated HTTP/2 configuration changes, which are not directly required for implementing the gRPC audit endpoints feature. Consider separating license header updates and HTTP/2 configuration changes into distinct PRs to maintain scope focus on gRPC audit endpoints implementation.
Docstring Coverage ⚠️ Warning Docstring coverage is 24.84% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'feat: lock gprc enpoints thought grpc bridge' contains a typo ('gprc' should be 'gRPC' and 'enpoints' should be 'endpoints' and 'thought' should be 'through'), making it unclear and imprecise. Correct the title to 'feat: lock gRPC endpoints through gRPC bridge' to accurately describe the gRPC audit endpoints feature being implemented.
✅ Passed checks (1 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR implements gRPC audit endpoints support by adding service implementations, configuration classes, protobuf definitions, and authorization handling, fully meeting the objective in issue #13062.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mo-auto
Copy link
Member

mo-auto commented Jan 6, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mo-auto mo-auto added comp-docs Touching folder /docs comp-jans-auth-server Component affected by issue or PR comp-jans-bom Component affected by issue or PR comp-jans-config-api Component affected by issue or PR comp-jans-core Component affected by issue or PR comp-jans-fido2 Component affected by issue or PR comp-jans-linux-setup Component affected by issue or PR comp-jans-lock comp-jans-scim Component affected by issue or PR kind-feature Issue or PR is a new feature request labels Jan 6, 2026
@mo-auto
Copy link
Member

mo-auto commented Jan 6, 2026

Error: Hi @yurem, You did not reference an open issue in your PR. I attempted to create an issue for you.
Please update that issues' title and body and make sure I correctly referenced it in the above PRs body.

yurem added 10 commits January 6, 2026 16:55
Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
final methods

Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
@yurem yurem marked this pull request as ready for review January 22, 2026 14:13
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 30

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (9)
jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/inject/CedarlingPolicy.java (1)

19-26: Consider enhancing the javadoc with usage description.

The javadoc currently only includes the author and date. Adding a brief description of the annotation's purpose and usage guidelines would improve code maintainability and help developers understand when to apply @CedarlingPolicy.

📝 Example javadoc enhancement
 /**
- * 
+ * Qualifier annotation for CDI injection of Cedarling policy components.
+ * Apply to injection points that require policy-specific bean selection.
+ *
  * `@author` Yuriy Movchan Date: 10/08/2022
  */
jans-core/service/src/main/java/io/jans/service/security/api/ProtectedApi.java (1)

15-20: Document grpcMethodName contract.

This is part of the public API; please add a brief Javadoc indicating expected format (e.g., fully qualified service/method name) and when it should be empty.

✍️ Suggested Javadoc
 	/**
      * `@return` Scopes access token should have to access this endpoint
      */
 	String[] scopes() default {};
 
+	/**
+     * `@return` gRPC method name mapped to this endpoint (empty if not applicable)
+     */
 	String grpcMethodName() default "";
jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/CedarlingProtectionService.java (1)

164-185: Dead code: id variable is computed but never used.

The id variable at lines 166-167 is calculated but never referenced in the resulting map. If it's intended for the entity mapping, it should be used; otherwise, remove the dead code.

Proposed fix (remove dead code)
 	private Map<String, Object> getCedarlingResource(CedarlingPermission requestedPermission) {
 		HashMap<String, Object> map = new HashMap<String, Object>();
-		int id = requestedPermission.hashCode();
-		id = id > 0 ? id : -id;
 		map.putAll(
jans-lock/lock-server/service/src/main/java/io/jans/lock/service/openid/OpenIdProtectionService.java (1)

41-41: Unused import.

The HttpHeaders import appears unused after the refactoring to accept bearerToken directly. Consider removing it.

Proposed fix
-import jakarta.ws.rs.core.HttpHeaders;
jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebService.java (1)

72-88: Align Cedarling path with bulk endpoints to prevent authorization failures.

The bulk endpoints at /audit/health/bulk, /audit/log/bulk, and /audit/telemetry/bulk each declare a non-bulk path in their @ProtectedCedarlingApi annotations. Since this path is passed directly to the Cedarling authorization engine (in CedarlingProtectionService.getCedarlingResource()), path-based authorization policies will check against the wrong route, causing authorization failures or unintended access.

Update the annotation paths to match the actual endpoint routes:

🔧 Suggested fix
- `@ProtectedCedarlingApi`(action = "Jans::Action::\"POST\"", resource = "Jans::HTTP_Request", id="lock_audit_health_write", path="/audit/health")
+ `@ProtectedCedarlingApi`(action = "Jans::Action::\"POST\"", resource = "Jans::HTTP_Request", id="lock_audit_health_write", path="/audit/health/bulk")
- `@ProtectedCedarlingApi`(action = "Jans::Action::\"POST\"", resource = "Jans::HTTP_Request", id="lock_audit_log_write", path="/audit/log")
+ `@ProtectedCedarlingApi`(action = "Jans::Action::\"POST\"", resource = "Jans::HTTP_Request", id="lock_audit_log_write", path="/audit/log/bulk")
- `@ProtectedCedarlingApi`(action = "Jans::Action::\"POST\"", resource = "Jans::HTTP_Request", id="lock_audit_telemetry_write", path="/audit/telemetry")
+ `@ProtectedCedarlingApi`(action = "Jans::Action::\"POST\"", resource = "Jans::HTTP_Request", id="lock_audit_telemetry_write", path="/audit/telemetry/bulk")

Applies to: lines 72-88, 108-124, 144-160.

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebServiceImpl.java (1)

200-212: Statistics reported before persistence confirmation.

Statistics are reported in lines 203-205 before the bulk persistence operation (line 206). If persistence fails, statistics will still reflect the entries. Depending on requirements, you may want to report stats only after successful persistence, or this may be acceptable for tracking activity regardless of storage outcome.

jans-config-api/plugins/docs/lock-plugin-swagger.yaml (3)

24-26: Typo: "Rerquest" should be "Request".

The summary and description contain "Rerquest" instead of "Request". Based on learnings, this Swagger file is auto-generated, so the fix should be applied in the source API code.


77-78: Typo: "Rerquest" should be "Request".

Same typo appears in the log records endpoint. Fix in the source API code.


130-132: Inconsistent summary/description: "Request" vs "Rerquest".

Line 130 correctly uses "Request" in the summary, but line 131 uses "Rerquest" in the description. Fix the typo in the source API code for consistency.

🤖 Fix all issues with AI agents
In
`@jans-core/service/src/main/java/io/jans/service/security/protect/BaseAuthorizationProtection.java`:
- Around line 12-13: Update the BaseAuthorizationProtection.processAuthorization
contract to explicitly document the expected bearerToken format and null/empty
handling: state whether bearerToken must be the raw access token (no "Bearer "
prefix) or may include the "Bearer " prefix and that implementations must strip
the prefix if present, and specify how null or empty values are treated (e.g.,
return a 401/unauthorized Response or throw a defined exception). Add this
Javadoc to the interface method processAuthorization(String bearerToken,
ResourceInfo resourceInfo) and ensure any implementations of
BaseAuthorizationProtection follow the documented behavior (strip prefix if
allowed, validate null/empty and return the documented Response).

In `@jans-linux-setup/jans_setup/schema/jans_schema.json`:
- Around line 4081-4085: The schema rename removed backward compatibility for
the attribute; restore it by adding the legacy name "severetyLevel" as an alias
(or include it in the "names" array) alongside the new "severityLevel" entry in
jans_schema.json so both names map to the same attribute definition (or,
alternatively, implement and reference an explicit migration step to transform
existing "severetyLevel" data to "severityLevel"); update the attribute
definition that currently lists "severityLevel" to include "severetyLevel" as
well so existing deployments continue to work.

In `@jans-linux-setup/jans_setup/setup_app/data/jetty_app_configuration.json`:
- Line 83: The jetty modules list currently enables cleartext HTTP/2 via
"http2c", exposing gRPC audit traffic; update configuration so http2c is not
enabled by default (remove or conditionally add "http2c" based on a new flag) OR
enable TLS by default in the GrpcConfiguration by setting useTls = true and
populating tlsCertChainFilePath and tlsPrivateKeyFilePath (and any related TLS
options) so BRIDGE-mode gRPC traffic is encrypted; reference the "modules" entry
(remove/guard "http2c") and the GrpcConfiguration fields useTls,
tlsCertChainFilePath to locate where to change settings and ensure documentation
notes the requirement if you choose to keep http2c behind an explicit opt-in.

In `@jans-linux-setup/jans_setup/setup_app/installers/jans_auth.py`:
- Around line 59-61: The jetty modules string in
self.jetty_app_configuration[self.service_name]['jetty']['modules'] is being
mutated with "+= ',http2c'", which can append duplicate "http2c" entries across
repeated install() calls; before appending, check the existing modules (split by
commas and strip whitespace) and only add "http2c" if it's not already present,
then join back into the modules string and proceed to call
self.install_jettyService(...).

In `@jans-linux-setup/jans_setup/templates/apache/https_jans.conf.mako`:
- Around line 166-167: In the apache template block guarded by
context.get('install_jans_lock') (the ProxyPass line in https_jans.conf.mako),
remove the stray trailing single-quote character at the end of the ProxyPass
directive so the URL token ends with /api/v1/configuration rather than
/api/v1/configuration'; update the ProxyPass entry for
/.well-known/lock-server-configuration to use the correct URL token (no trailing
quote) to avoid Apache parse errors.
- Around line 147-151: The current directives use both "Require all granted" and
"Require method GET POST", which are OR'd and make the method restriction
ineffective; either remove the "Require all granted" directive or wrap both in a
"<RequireAll>" container so that access is allowed only for GET and POST. Locate
the block containing "Require all granted" and "Require method GET POST" in
https_jans.conf.mako and apply one of the two fixes: delete the "Require all
granted" line if only method restriction is intended, or replace the two
standalone Require lines with a "<RequireAll>" container enclosing both
requirements to enforce both conditions.

In
`@jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/CedarlingProtectionService.java`:
- Around line 87-89: In CedarlingProtectionService (the block checking
requestedPermissions), replace the non-idiomatic check
"requestedPermissions.size() == 0" with the idiomatic collection check
"requestedPermissions.isEmpty()" so the conditional reads e.g. if
(requestedPermissions.isEmpty()) { return simpleResponse(INTERNAL_SERVER_ERROR,
"Access to operation is not correct"); } to improve readability and performance.
- Around line 82-83: The code in CedarlingProtectionService currently removes
the "Bearer" prefix using a case-sensitive regex and logs the full bearerToken
(log.debug("Validating token {}", bearerToken)), which risks auth failures and
credential leakage; update the prefix removal to use a case-insensitive pattern
like "(?i)Bearer\\s+" (matching other extractBearerToken implementations) and
remove or replace the debug call so it does not emit the full token (e.g., log
only that a token was received or log a masked/hashed token) while keeping the
variable name bearerToken and the validation flow intact.

In
`@jans-lock/lock-server/model/src/main/java/io/jans/lock/model/audit/HealthEntry.java`:
- Around line 131-147: The equals() and hashCode() implementations in
HealthEntry currently omit the inum field (which is annotated ignoreDuringUpdate
= true and likely serves as the unique identifier), so add inum to both equality
checks and the hash calculation to ensure entries with different inum are not
considered equal; specifically update HealthEntry.equals to include
Objects.equals(inum, that.inum) in the returned conjunction and update
HealthEntry.hashCode to include inum in the Objects.hash(...) call (or
alternatively, if content-only equality is desired, add a clear Javadoc on
HealthEntry explaining why inum is intentionally excluded).
- Around line 70-73: The engineStatus field in HealthEntry is missing an
explicit Jackson mapping; add the `@JsonProperty`("engineStatus") annotation to
the engineStatus field (which already has `@JsonObject` and `@AttributeName`(name =
"engineStatus")) so its JSON name is explicit and consistent with the other
fields in the HealthEntry class.

In
`@jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/BaseDnConfiguration.java`:
- Around line 1-7: Update the copyright year in the file containing class
BaseDnConfiguration from 2024 to the correct year (e.g., 2026) to reflect the PR
creation date; locate the header comment block at the top of the file (the
multi-line comment preceding the class declaration) and change the "(c) 2024,
Janssen Project" text to "(c) 2026, Janssen Project" (or the appropriate year),
keeping the existing license text intact.

In
`@jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/Conf.java`:
- Line 1: The file's top-of-file comment opener is invalid ("/*/*") causing a
compilation error; in Conf.java (the Conf class/source), replace the malformed
opener with a standard block comment opener (e.g., "/*") so the comment starts
correctly and no stray tokens remain—ensure the rest of the header comment and
closing "*/" remain intact.

In
`@jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/GrpcServerMode.java`:
- Around line 9-37: GrpcServerMode currently only defines `@JsonValue` getMode()
which serializes the enum but does not provide a way for Jackson to deserialize
strings like "bridge" or "plain_server"; add a static factory annotated with
`@JsonCreator` (e.g., public static GrpcServerMode fromString(String mode)) that
maps the incoming string to the correct enum constant (matching the mode field,
case- and null-safe, and throwing or returning a default if unmatched) so
DataMapperService.readValue() can deserialize JSON into GrpcServerMode
correctly.

In `@jans-lock/lock-server/service/pom.xml`:
- Around line 66-85: The protobuf-maven-plugin block hardcodes protoc and grpc
plugin versions (protocArtifact and pluginArtifact); change these to use
BOM-managed properties (e.g. refer to ${protobuf.version} and ${grpc.version} or
the existing BOM property used for protobuf-java) instead of literal "4.33.2"
and "1.78.0" in the protobuf-maven-plugin configuration, and add a
protobuf.version property to the jans-bom (or reuse the protobuf-java property
at line 827) so the plugin picks up the BOM-managed version; keep the pluginId
and execution (id "compile_proto") unchanged.
- Around line 164-172: The POM currently declares the javax.servlet-api
dependency which conflicts with grpc-servlet-jakarta and the project's Jakarta
APIs; remove the <artifactId>javax.servlet-api</artifactId> dependency
declaration and either replace it with the Jakarta servlet artifact
(<artifactId>jakarta.servlet-api</artifactId>) if you need a compile-time
provided stub or simply delete the dependency so the Jakarta EE runtime provides
it, ensuring grpc-servlet-jakarta is used consistently with the jakarta.servlet
namespace.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/filter/AuthorizationProcessingFilter.java`:
- Around line 98-99: The file is missing a blank line between method
definitions; add one empty line after the end of the extractBearerToken() method
so that there is a blank line before the declaration of
unprotectedApiResponse(String name) to match the file's formatting conventions.
- Around line 90-98: The Bearer token extraction logic in
AuthorizationProcessingFilter.extractBearerToken() is duplicated in
CedarlingAuthorizationProcessingFilter; update HeaderUtils.extractBearerToken()
so it performs the same case-insensitive "Bearer\\s+" removal but returns null
when the Authorization header is missing or doesn't start with a Bearer token,
then replace AuthorizationProcessingFilter.extractBearerToken() and the
corresponding method in CedarlingAuthorizationProcessingFilter to call
HeaderUtils.extractBearerToken() instead of duplicating the logic.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/security/GrpcAuthorizationInterceptor.java`:
- Around line 94-123: The code calls extractResourceInfo(methodName) and passes
the possibly null ResourceInfo into
authorizationProtection.processAuthorization, which can cause NPEs or improper
authorization; update the interceptor to check the result of extractResourceInfo
and fail-closed when it's null: if ResourceInfo resourceInfo is null, log a
warning including methodName, perform audit logging as a denied request (use
AuditLogEntry and applicationAuditLogger.log with success=false), map an
appropriate HTTP-to-gRPC status and close the call with call.close(...), then
return an empty ServerCall.Listener; only call processAuthorization and
Contexts.interceptCall when resourceInfo is non-null. Ensure references:
extractResourceInfo, ResourceInfo, processAuthorization, AuditLogEntry,
applicationAuditLogger, mapHttpStatusToGrpcStatus, methodName, call, and next.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/server/GrpcServerStarter.java`:
- Around line 67-103: The startup ignores GrpcConfiguration.useTls; update
startGrpcServer to derive an effective mode (e.g., effectiveMode) using both
grpcConfiguration.getServerMode() and grpcConfiguration.isUseTls() so that
useTls=true forces TLS behavior even if serverMode is BRIDGE/default, then
branch on effectiveMode (GrpcServerMode.TLS_SERVER vs PLAIN_SERVER) when
building the server; refer to startGrpcServer,
GrpcConfiguration.getServerMode(), GrpcConfiguration.isUseTls(), and
GrpcServerMode to locate where to compute the effective mode and apply it to the
Netty vs ServerBuilder logic.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/servlet/GrpcAuditServlet.java`:
- Around line 71-74: The log message in GrpcAuditServlet.initializeGrpc() has a
typo ("initializion"); update the log.info call in the initializeGrpc method to
use the correct spelling (e.g., "gRPC adapter initialization") so the
initialization log is spelled correctly and consistent with other messages.
- Around line 165-177: The current URI normalization in GrpcAuditServlet can
yield an empty or missing-leading-slash requestURI/servletPath when stripping
the contextPath (from request.getRequestURI()), which breaks gRPC method
resolution; after computing this.requestURI (and before setting
this.servletPath) ensure the stripped value is non-empty and starts with a
leading '/' (e.g., if substring yields "" or does not startWith("/"), prefix a
"/"), then set this.servletPath from the normalized this.requestURI (otherwise
fall back to request.getServletPath()); update the logic around
request.getRequestURI(), request.getContextPath(), this.requestURI and
this.servletPath accordingly.
- Around line 153-163: The GrpcRequestWrapper currently uses locale-sensitive
toLowerCase for HTTP header normalization and lookups which can break ASCII
headers (e.g., Turkish locale); update header normalization and lookup calls to
use Locale.ROOT: change headers.put(name.toLowerCase(Locale.ROOT), values) in
the GrpcRequestWrapper constructor and change
headers.get(name.toLowerCase(Locale.ROOT)) wherever header lookups occur (e.g.,
the methods referencing the map at the two noted retrieval sites), and add the
import for java.util.Locale; this ensures locale-insensitive lowercasing for
header names.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/openid/OpenIdProtection.java`:
- Around line 13-16: OpenIdProtection currently redeclares processAuthorization
which is already inherited from BaseAuthorizationProtection; remove the
redundant method declaration (processAuthorization(String bearerToken,
ResourceInfo resourceInfo)) from the OpenIdProtection interface so it simply
extends BaseAuthorizationProtection without re-declaring the inherited method.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/openid/OpenIdProtectionService.java`:
- Line 93: The code in OpenIdProtectionService currently strips the bearer
prefix using a case-sensitive regex ("Bearer\\s+"), causing failure for
lowercase variations; change the replacement to use a case-insensitive pattern
(e.g., use the "(?i)Bearer\\s+" regex) when modifying bearerToken so it matches
"Bearer", "bearer", etc., to be consistent with
AuthorizationProcessingFilter.extractBearerToken().

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebServiceImpl.java`:
- Around line 366-368: There's a typo in the StatService method name: change
reportOpearation to reportOperation across the codebase; update the StatService
interface declaration, all implementing classes (e.g., classes implementing
StatService), and every call site such as AuditRestWebServiceImpl where
statService.reportOpearation(...) is invoked so they call
statService.reportOperation(...). Ensure method signatures remain the same aside
from the corrected name and run/adjust any package-level imports or references
affected by the rename.
- Around line 411-443: persistBulkAuditData currently iterates and calls
auditService.addLogEntry/addHealthEntry/addTelemetryEntry one-by-one, which is
non-atomic and inefficient; change this by either (A) adding batch methods on
AuditService (e.g. addLogEntries(List<LogEntry>),
addHealthEntries(List<HealthEntry>), addTelemetryEntries(List<TelemetryEntry>))
and have persistBulkAuditData call those batch methods instead of per-entry
calls, or (B) wrap the persistBulkAuditData operation in a transactional
boundary (e.g., annotate persistBulkAuditData or its caller with `@Transactional`)
and ensure AuditService implementations participate in the transaction so all
entries are committed or rolled back together. Ensure corresponding AuditService
interface and implementations are updated to support the chosen approach and
handle errors consistently.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/util/HeaderUtils.java`:
- Around line 62-74: The method in HeaderUtils that extracts bearer tokens
currently checks trimmed.toLowerCase().startsWith("bearer ") and then
redundantly checks trimmed.startsWith("Bearer "), and finally returns the header
as-is for non-bearer schemes; change it so you remove the second redundant
startsWith check (the toLowerCase check already covers case variants) and change
the non-bearer fallback to return null (not the original header) so only bearer
tokens are extracted (keep the existing substring(7).trim() logic when a bearer
prefix is detected).

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/util/ServerUtil.java`:
- Around line 141-170: In getGrpcClientIpAddress(io.grpc.ServerCall<?, ?> call,
Metadata headers) change the lookup order to prefer the socket address from
call.getAttributes() (io.grpc.Grpc.TRANSPORT_ATTR_REMOTE_ADDR) before trusting
client-controllable headers (getIpFromHeaders), and only consult headers when a
trusted-proxy configuration is enabled; also make the socket-address extraction
null-safe by not calling InetSocketAddress.getAddress().getHostAddress()
directly—use InetSocketAddress.getHostString() as a safe fallback or check
getAddress() for null before calling getHostAddress() to avoid NPEs; update any
related logic in GrpcAuthorizationInterceptor that relies on this method to
ensure authorization uses the socket-derived IP unless proxy trust is explicitly
configured.

In `@jans-lock/lock-server/service/src/main/proto/grpc/audit.proto`:
- Around line 79-83: The shared AuditResponse message is too coarse for bulk
RPCs; update the proto definition for AuditResponse (used by RPCs handling
multiple records) to include granular bulk-processing fields such as int32
processed_count, int32 failed_count and repeated AuditFailure failures (define
AuditFailure with index/id and string error_message), and optionally a repeated
string failed_ids or map<string,string> failure_details so callers can
distinguish partial successes and inspect per-item errors while preserving
existing success/message fields.
- Around line 35-38: Replace the open string fields with proto enums: define
enums (e.g., EventType, SeverityLevel, DecisionResult, and EntryStatus) in this
proto and change the message fields event_type, severity_level, decision_result
and the status fields in HealthEntry/TelemetryEntry to use those enum types
instead of string; ensure each enum has explicit numeric values and update any
generated code or usages to refer to the new enum symbols (EventType,
SeverityLevel, DecisionResult, EntryStatus) so callers use the typed enum values
rather than freeform strings.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (7)
jans-lock/lock-server/model/src/main/java/io/jans/lock/model/core/LockApiError.java (1)

58-60: Optional: Fix parameter name typo.

The parameter name messge is a typo (should be message). While this pre-existing issue doesn't affect functionality, correcting it would improve code readability and maintainability.

✏️ Suggested fix
-    public ErrorBuilder withMessage(String messge) {
-        this.message = messge;
+    public ErrorBuilder withMessage(String message) {
+        this.message = message;
         return this;
     }
jans-core/service/src/main/java/io/jans/service/security/api/ProtectedApi.java (1)

15-20: Add Javadoc for the new annotation element.

Keeps the annotation self-describing alongside scopes().

✍️ Suggested doc addition
-    String grpcMethodName() default "";
+    /**
+     * `@return` gRPC method name (e.g., fully-qualified) for protection mapping.
+     */
+    String grpcMethodName() default "";
jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/AppConfiguration.java (1)

354-367: Update toString() to include grpcConfiguration.

The new grpcConfiguration field is not included in toString(), which will make debugging and log analysis incomplete when gRPC configuration is involved.

♻️ Proposed fix
 	`@Override`
 	public String toString() {
 		return "AppConfiguration [baseDN=" + baseDN + ", baseEndpoint=" + baseEndpoint + ", openIdIssuer="
 				+ openIdIssuer + ", protectionMode=" + protectionMode + ", auditPersistenceMode=" + auditPersistenceMode
-				+ ", cedarlingConfiguration=" + cedarlingConfiguration + ", statEnabled=" + statEnabled
+				+ ", cedarlingConfiguration=" + cedarlingConfiguration + ", grpcConfiguration=" + grpcConfiguration
+				+ ", statEnabled=" + statEnabled
 				+ ", statTimerIntervalInSeconds=" + statTimerIntervalInSeconds + ", tokenChannels=" + tokenChannels
 				+ ", clientId=" + clientId + ", clientPassword=" + clientPassword + ", disableJdkLogger="
jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/filter/CedarlingAuthorizationProcessingFilter.java (1)

100-108: Remove unused methods unprotectedApiResponse and disabledApiResponse.

These methods are defined but never called within this class. They appear to be leftover from copy-paste or template code. Remove them to reduce dead code in the codebase.

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/openid/OpenIdProtectionService.java (1)

41-41: Consider removing unused import.

The HttpHeaders import on line 41 appears to be unused after the processAuthorization method signature was refactored to accept String bearerToken directly instead of HttpHeaders.

♻️ Proposed fix
 import jakarta.ws.rs.container.ResourceInfo;
-import jakarta.ws.rs.core.HttpHeaders;
 import jakarta.ws.rs.core.Response;
jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/CedarlingProtectionService.java (1)

164-185: Remove unused variable id.

The local variable id computed from requestedPermission.hashCode() is never used in the method body. The actual ID value comes from requestedPermission.getId() on line 170.

♻️ Proposed fix
 	private Map<String, Object> getCedarlingResource(CedarlingPermission requestedPermission) {
 		HashMap<String, Object> map = new HashMap<String, Object>();
-		int id = requestedPermission.hashCode();
-		id = id > 0 ? id : -id;
 		map.putAll(
 				Map.of("cedar_entity_mapping",
 						Map.of("entity_type", requestedPermission.getResource(), "id", requestedPermission.getId())
jans-config-api/plugins/docs/lock-plugin-swagger.yaml (1)

841-867: Add a description for disableExternalLoggerConfiguration.
The new flag is undocumented in the schema; adding a short description will keep the docs self-explanatory. If this file is generated, please update the source-of-truth that emits it.

📝 Suggested doc tweak
         disableExternalLoggerConfiguration:
           type: boolean
+          description: Disable loading external logger configuration from filesystem
🤖 Fix all issues with AI agents
In `@jans-bom/pom.xml`:
- Around line 27-30: The pom declares <grpc.version>1.78.0</grpc.version> while
resteasy-grpc-bom:1.0.0.Final manages protoc-gen-grpc-java at 1.73.0; to fix,
run a full build and runtime smoke tests to validate gRPC 1.78.0 with
resteasy-grpc-bom 1.0.0.Final, and if you observe classpath or plugin
incompatibilities, either align versions by setting the protoc-gen-grpc-java
plugin version to 1.78.0 (override the plugin managed by resteasy-grpc-bom) or
downgrade grpc.version to 1.73.0 so they match; locate these properties in the
pom (grpc.version, grpc-bridge.version, and the resteasy-grpc-bom import) and
update or add the plugin override accordingly, then re-run CI and integration
tests to confirm no conflicts.

In `@jans-lock/lock-server.yaml`:
- Around line 694-700: The policyStats schema repeats the same description
inside additionalProperties; remove the description field from the
additionalProperties block and keep the single description on the parent
policyStats object to eliminate redundancy (target symbols: policyStats and its
additionalProperties entry).

In
`@jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/config/BootstrapConfig.java`:
- Around line 1-5: The license header in class BootstrapConfig currently uses
the non-standard phrase "Apache License (2004)"; update the header comment to
use the standard reference "Apache License, Version 2.0" (or include the SPDX
identifier "Apache-2.0") and ensure the formatting matches other project source
files—replace the existing line in the top-of-file comment block in
BootstrapConfig.java accordingly.

In
`@jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/filter/CedarlingAuthorizationProcessingFilter.java`:
- Around line 90-98: extractBearerToken currently returns the raw Authorization
header when it doesn't contain a Bearer token (e.g., "Basic ..."), which then
propagates invalid values to processAuthorization; update extractBearerToken to
explicitly validate the Bearer scheme before returning a token by checking the
header matches the Bearer pattern (case-insensitive) or starts with "Bearer "
ignoring case, then extract and trim the token, otherwise return null so only
valid Bearer tokens reach processAuthorization.

In
`@jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/inject/CedarlingPolicy.java`:
- Around line 1-5: Update the file header in CedarlingPolicy.java to use the
standard license name "Apache License, Version 2.0" instead of "Apache License
(2004)" and adjust the copyright years to reflect the creation-to-current span
(e.g., "2022-2025" or "2022, 2025"); modify only the file comment block at the
top of CedarlingPolicy.java to make these textual changes so the license and
copyright statements are accurate and standard.

In
`@jans-lock/lock-server/model/src/main/java/io/jans/lock/model/audit/LogEntry.java`:
- Around line 217-242: The equals() and hashCode() implementations in LogEntry
currently omit the immutable persisted identifier inum (and the inherited DN),
causing distinct persisted entries to compare equal; update
LogEntry.equals(Object o) to include Objects.equals(inum, logEntry.inum) (and if
you rely on the parent Entry DN, include Objects.equals(getDn(),
logEntry.getDn()) or the dn field) in the chain of comparisons, and add inum
(and dn/getDn()) to the Objects.hash(...) call in LogEntry.hashCode() so
identity includes the persisted identifier.

In
`@jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/cedarling/LogLevel.java`:
- Around line 2-4: Update the license header in LogLevel.java to use the
standard Apache phrasing: replace "Apache License (2004)" with "Apache License,
Version 2.0" and use the canonical URL
"https://www.apache.org/licenses/LICENSE-2.0"; ensure the header matches the
project's standard template (same wording, copyright line format) for the
LogLevel class so future files are consistent.

In
`@jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/Conf.java`:
- Line 7: Remove the extra blank line after the license header in Conf.java so
the file follows Java formatting conventions; ensure there is a single blank
line (or none if header ends immediately) before the package declaration
"package io.jans.lock.model.config;" so the file header and package statement
are directly adjacent and consistently formatted.

In
`@jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/grpc/GrpcConfiguration.java`:
- Around line 27-29: The grpcPort field in GrpcConfiguration allows any int but
should be constrained to valid TCP ports (1–65535); add validation to prevent
invalid values by annotating the grpcPort field with
javax.validation.constraints.Min(1) and javax.validation.constraints.Max(65535)
and/or add a guard in the GrpcConfiguration.setGrpcPort(int) setter that throws
IllegalArgumentException for values outside 1–65535; update the field
documentation if needed so the schema and runtime validation both enforce the
valid port range.
- Around line 31-42: Add validation to GrpcConfiguration to enforce TLS config
consistency: when the boolean field useTls is true, ensure tlsCertChainFilePath
and tlsPrivateKeyFilePath are non-blank and throw a clear IllegalStateException
if either is missing; implement this as a public validate() method on the
GrpcConfiguration class (or annotate with Bean Validation if preferred) and call
validate() before gRPC server startup so fields useTls, tlsCertChainFilePath,
and tlsPrivateKeyFilePath are checked early.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/audit/GrpcAuditServiceImpl.java`:
- Line 56: Change the routine gRPC invocation logs in GrpcAuditServiceImpl from
INFO to DEBUG to reduce verbosity: replace log.info(...) with log.debug(...) in
the methods processHealth, processBulkHealth, processLog, processBulkLog,
processTelemetry, and processBulkTelemetry; keep INFO-level logs only for
significant events (errors or notable state changes) and ensure the same log
message text is preserved when switching to DEBUG.
- Around line 65-68: The catch blocks in GrpcAuditServiceImpl currently call
responseObserver.onError(e) with raw exceptions (and log.error(..., e)), which
can leak internal stack traces to gRPC clients; change those catch handlers to
log the exception internally (using log.error) but pass a wrapped
StatusRuntimeException to responseObserver.onError, e.g., create a
StatusRuntimeException via Status.INTERNAL.withDescription("Error processing
<operation>").withCause(null).asRuntimeException() or similar (include minimal
safe description), and replace direct responseObserver.onError(e) calls with
responseObserver.onError(wrappedException); apply the same pattern to every
catch block in GrpcAuditServiceImpl, ensuring you reference
responseObserver.onError and use Status.*.asRuntimeException() to create the
wrapper.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/audit/GrpcAuditServiceProvider.java`:
- Around line 39-48: The init() method in GrpcAuditServiceProvider should
validate injected dependencies before using them: check that
auditRestWebService, mapper, and log are not null and throw a clear
IllegalStateException (or log an explicit error) if any are null, then only
instantiate grpcAuditService = new GrpcAuditServiceImpl(...). Add these null
checks at the start of init() to fail-fast with a descriptive message
referencing the missing dependency names so the cause is clear when CDI
injection fails.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/security/GrpcAuthorizationInterceptor.java`:
- Around line 65-66: The constant AUTHORIZATION_METADATA_KEY in
GrpcAuthorizationInterceptor is unused (the code calls
HeaderUtils.findAndExtractBearerToken(headers) instead); remove the unused field
declaration to clean up the class and imports, and ensure no other references
remain to AUTHORIZATION_METADATA_KEY after deletion so compilation is
unaffected.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/server/GrpcServerStarter.java`:
- Around line 140-165: The stopGrpcServer method leaves the grpcServer field
non-null after shutdown, which can cause stale state on later restarts; update
stopGrpcServer to set grpcServer = null after a successful shutdown path and
also after the InterruptedException handling (i.e., after forcing shutdown in
both the normal and catch paths) so that the grpcServer reference is cleared for
future reinitialization; locate the method stopGrpcServer and the grpcServer
field to add the null assignment in all exit paths.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/OpenIdService.java`:
- Around line 2-4: Update the file header in OpenIdService.java to use the
standardized Apache License, Version 2.0 header used elsewhere (e.g.,
ConfigurationService.java) and change the copyright year to 2025 (or use the
range "2022-2025" if you prefer); specifically replace the existing top-of-file
block comment with the current project license template and updated year(s) so
the class OpenIdService reflects the consistent license header format across the
codebase.

In `@jans-lock/lock-server/service/src/main/proto/grpc/audit.proto`:
- Around line 1-9: The proto declares package io.jans.lock.audit (and outer
classname AuditProto) but the file lives outside the expected io/jans/lock/audit
directory, causing Buf lint failures; fix by either moving this proto file into
the repository proto root subdirectory path io/jans/lock/audit so the filesystem
matches the declared package, or update the package declaration to match the
current directory layout (and update references to AuditProto accordingly);
alternatively, if you intentionally keep the current layout, add/configure
buf.yaml to allow this directory mapping so Buf won’t reject files with package
io.jans.lock.audit.
♻️ Duplicate comments (29)
jans-lock/lock-server/service/src/main/java/io/jans/lock/service/filter/AuthorizationProcessingFilter.java (1)

90-97: Validate Bearer prefix and reject non-token headers before passing downstream.

Currently a non‑Bearer Authorization header (or whitespace-only) is forwarded unchanged, which can bypass expected token semantics and lead to inconsistent auth behavior. Ensure only Bearer tokens are accepted and blank tokens return null.

🔧 Proposed fix (strict Bearer parsing)
     private String extractBearerToken() {
         String authHeader = httpHeaders.getHeaderString(HttpHeaders.AUTHORIZATION);
-        
-        if (StringUtils.isEmpty(authHeader)) {
-            return null;
-        }
-
-        return authHeader.replaceFirst("(?i)Bearer\\s+", "");
+        if (StringUtils.isBlank(authHeader)) {
+            return null;
+        }
+        if (!StringUtils.startsWithIgnoreCase(authHeader, "Bearer ")) {
+            return null;
+        }
+        String token = authHeader.substring(7).trim();
+        return StringUtils.isBlank(token) ? null : token;
     }
jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/Conf.java (1)

1-1: Fix invalid comment opener to avoid compilation failure.

The malformed opener /*/* on Line 1 will cause a Java syntax error: the first /* opens a block comment, the */ closes it immediately, leaving a stray * token that prevents compilation.

🛠️ Proposed fix
-/*/*
+/*
jans-linux-setup/jans_setup/setup_app/installers/jans_auth.py (1)

59-60: Potential duplicate module addition on repeated calls.

The string concatenation += ',http2c' mutates jetty_app_configuration in-place. If install() is called multiple times, http2c will be appended repeatedly, resulting in invalid module strings like ...,http2c,http2c.

Guard against duplicate additions by checking if http2c is already present before appending.

jans-linux-setup/jans_setup/setup_app/data/jetty_app_configuration.json (1)

82-84: Confirm h2c exposure is intentional for gRPC audit bridge, or enable TLS by default.

http2c enables cleartext HTTP/2, which is used as the transport for the gRPC audit servlet. This leaves audit traffic unencrypted when jans-lock is not behind a TLS-terminating proxy. Either document the requirement for external TLS termination or enable TLS in gRPC configuration by default.

jans-linux-setup/jans_setup/templates/apache/https_jans.conf.mako (2)

147-151: Conflicting access control directives render method restriction ineffective.

Require all granted (line 148) unconditionally allows access, making Require method GET POST (line 151) redundant. In Apache 2.4, multiple Require directives are OR'd by default.

Remove Require all granted if method restriction is intended, or use a <RequireAll> container.


166-167: Syntax error: trailing single quote in ProxyPass directive.

Line 167 has a trailing single quote (') that will cause Apache configuration parsing to fail:

ProxyPass   /.well-known/lock-server-configuration http://localhost:${lock_host_port}/${lock_host_suffix}/api/v1/configuration'

Remove the trailing quote.

jans-linux-setup/jans_setup/schema/jans_schema.json (1)

4081-4085: Preserve backward compatibility for the renamed attribute.

Existing deployments that still use severetyLevel will break if the legacy alias is removed. Please keep both names.

🩹 Proposed compatibility fix
       "names": [
-        "severityLevel"
+        "severityLevel",
+        "severetyLevel"
       ],
jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/BaseDnConfiguration.java (1)

2-4: Update the copyright year to the current range.

The header still shows 2024; this PR is dated January 2026.

📅 Proposed fix
- * Copyright (c) 2024, Janssen Project
+ * Copyright (c) 2026, Janssen Project
jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/GrpcServerMode.java (1)

9-37: Add @JsonCreator to support JSON deserialization.

@JsonValue handles serialization only; without a creator or mapper settings, values like "bridge" may fail to deserialize.

🛠️ Suggested fix
 import com.fasterxml.jackson.annotation.JsonValue;
+import com.fasterxml.jackson.annotation.JsonCreator;
 
 public enum GrpcServerMode {
     DISABLED("disabled"), BRIDGE("bridge"), PLAIN_SERVER("plain_server"), TLS_SERVER("tls_server");
@@
     private final String mode;
 
     private GrpcServerMode(String mode) {
         this.mode = mode;
     }
+
+    `@JsonCreator`
+    public static GrpcServerMode fromString(String value) {
+        if (value == null) {
+            return null;
+        }
+        for (GrpcServerMode mode : values()) {
+            if (mode.mode.equalsIgnoreCase(value)) {
+                return mode;
+            }
+        }
+        throw new IllegalArgumentException("Unsupported GrpcServerMode: " + value);
+    }
@@
     `@JsonValue`
     public String getMode() {
         return mode;
     }
 }
#!/bin/bash
# Check mapper settings for enum deserialization
rg -n "READ_ENUMS_USING_TO_STRING|ACCEPT_CASE_INSENSITIVE_ENUMS|ObjectMapper" --type java
jans-core/service/src/main/java/io/jans/service/security/protect/BaseAuthorizationProtection.java (1)

12-13: Clarify bearer token format and null/empty handling in the contract.
Add Javadoc specifying whether bearerToken is raw (no “Bearer ” prefix) and how null/empty should be handled so implementations are consistent.

♻️ Suggested Javadoc
 public interface BaseAuthorizationProtection {
+    /**
+     * `@param` bearerToken raw bearer token value (without "Bearer " prefix), or null/empty if absent
+     * `@param` resourceInfo JAX-RS resource info for the target endpoint
+     * `@return` authorization result
+     */
     Response processAuthorization(String bearerToken, ResourceInfo resourceInfo);
 }
jans-lock/lock-server/service/src/main/java/io/jans/lock/service/openid/OpenIdProtection.java (1)

13-16: Remove the redundant method declaration.
processAuthorization is already inherited from BaseAuthorizationProtection; re-declaring it is unnecessary and risks drift.

♻️ Proposed cleanup
 public interface OpenIdProtection extends BaseAuthorizationProtection {
-
-    Response processAuthorization(String bearerToken, ResourceInfo resourceInfo);
-
     public static Response simpleResponse(Response.Status status, String detail) {
         return Response.status(status).entity(detail).build();
     }
jans-lock/lock-server/service/src/main/java/io/jans/lock/util/HeaderUtils.java (1)

54-73: Return null for non-bearer schemes and drop the redundant check.
extractBearerToken currently returns the header as-is for non-bearer schemes and repeats a case check; that conflicts with its contract (“null if invalid”).

🐛 Proposed fix
     public static String extractBearerToken(String authHeader) {
         if (authHeader == null || authHeader.trim().isEmpty()) {
             return null;
         }
         
         // Remove any extra whitespace
         String trimmed = authHeader.trim();
         
-        // Check if it's a bearer token
-        if (trimmed.toLowerCase().startsWith("bearer ")) {
-            return trimmed.substring(7).trim(); // Remove "bearer " prefix
-        }
-        
-        // Also accept "Bearer " with capital B
-        if (trimmed.startsWith("Bearer ")) {
-            return trimmed.substring(7).trim();
-        }
-        
-        // If it doesn't start with bearer, return as-is (might be basic auth or other)
-        return trimmed;
+        // Check if it's a bearer token (case-insensitive)
+        if (trimmed.toLowerCase().startsWith("bearer ")) {
+            return trimmed.substring(7).trim();
+        }
+        
+        // Not a bearer token
+        return null;
     }
jans-lock/lock-server/service/src/main/java/io/jans/lock/util/ServerUtil.java (1)

144-172: Fix null pointer dereference and reverse IP lookup priority for security.

This is a duplicate of a previous review comment that remains unaddressed:

  1. NPE risk (Line 159): inetAddr.getAddress() returns null for unresolved addresses, causing NullPointerException on .getHostAddress().

  2. Security issue: The method prioritizes client-controllable headers (X-Forwarded-For, X-Real-IP) over the trusted socket address. Since this IP is used for authorization in GrpcAuthorizationInterceptor, header spoofing is a security risk unless behind a trusted proxy.

🔧 Suggested fix for null-safety and secure priority
     public static String getGrpcClientIpAddress(io.grpc.ServerCall<?, ?> call, Metadata headers) {
         try {
-            // Method 1: Try to get from metadata headers
-            String ipFromHeaders = getIpFromHeaders(headers);
-            if (ipFromHeaders != null) {
-                log.debug("Client IP from headers: {}", ipFromHeaders);
-                return ipFromHeaders;
-            }
-
-            // Method 2: Get from ServerCall attributes
+            // Method 1: Prefer trusted socket address
             if (call != null) {
                 io.grpc.Attributes attributes = call.getAttributes();
                 java.net.SocketAddress remoteAddr = attributes.get(io.grpc.Grpc.TRANSPORT_ATTR_REMOTE_ADDR);
                 if (remoteAddr instanceof java.net.InetSocketAddress) {
                     java.net.InetSocketAddress inetAddr = (java.net.InetSocketAddress) remoteAddr;
-                    String ip = inetAddr.getAddress().getHostAddress();
+                    java.net.InetAddress addr = inetAddr.getAddress();
+                    String ip = (addr != null) ? addr.getHostAddress() : inetAddr.getHostString();
                     log.debug("Client IP from ServerCall attributes: {}", ip);
                     return ip;
                 }
             }
 
+            // Method 2: Fallback to headers (only if trusted proxy is configured)
+            String ipFromHeaders = getIpFromHeaders(headers);
+            if (ipFromHeaders != null) {
+                log.debug("Client IP from headers: {}", ipFromHeaders);
+                return ipFromHeaders;
+            }
+
             log.warn("Could not determine client IP address");
             return "unknown";
jans-lock/lock-server/model/src/main/java/io/jans/lock/model/audit/HealthEntry.java (2)

70-73: Missing @JsonProperty annotation on engineStatus field.

Unlike other fields in this class, engineStatus lacks the @JsonProperty annotation. While Jackson uses the field name by default, adding the annotation maintains consistency and explicit API contract.

Suggested fix
 	// Details: cedarEngineStatus, cedarPolicyStatus, tokenDataStatus. etc..
+	`@JsonProperty`("engineStatus")
 	`@JsonObject`
 	`@AttributeName`(name = "engineStatus")
 	private Map<String, String> engineStatus;

131-147: Consider including inum in equals() and hashCode().

The inum field is excluded from equality checks. If this is intentional (content-based equality), consider documenting this design decision with a comment. Otherwise, two entries with different inum values but identical content will be considered equal.

jans-lock/lock-server/service/src/main/proto/grpc/audit.proto (2)

35-38: Consider using enums for constrained value fields.

Fields like severity_level, decision_result, and status represent finite known values. Using proto enums would provide compile-time type safety and self-documenting allowed values. This was flagged in a previous review.


79-83: Consider adding granularity for bulk operation responses.

The current AuditResponse is shared across single and bulk RPCs. For bulk operations, adding fields like processed_count, failed_count, and optional error details would help clients handle partial success scenarios. This was flagged in a previous review.

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebServiceImpl.java (2)

366-368: Typo in method name: reportOpearation.

The method statService.reportOpearation contains a typo ("Opearation" instead of "Operation"). This was flagged in a previous review.


411-443: Non-atomic bulk persistence may leave data in inconsistent state.

The bulk persistence methods iterate entries individually without transactional wrapping. If any entry fails, preceding entries remain persisted. This was flagged in a previous review.

Consider either adding batch methods to AuditService or wrapping in a transaction boundary.

jans-lock/lock-server/service/pom.xml (2)

164-172: Servlet API namespace conflict: grpc-servlet-jakarta requires Jakarta namespace.

This configuration declares javax.servlet-api while using grpc-servlet-jakarta (line 241). The grpc-servlet-jakarta artifact is compiled against jakarta.servlet namespace (Jakarta EE 9+), not javax.servlet. Mixing these namespaces will cause runtime class loading failures when gRPC servlet integration attempts to use Jakarta servlet types.

The comment at line 164-166 acknowledges the concern but the fix is incorrect—remove javax.servlet-api and rely on the Jakarta servlet API already provided by the container (or explicitly declare jakarta.servlet-api if needed at compile time).

🔧 Proposed fix
 		<!-- Servlet -->
-		<!-- Note: using javax.servlet-api for compatibility with grpc-servlet; avoid mixing Jakarta and javax servlet APIs -->
-
-		<!-- Compatibility: some gRPC/servlet libs compiled against javax.servlet -->
-		<dependency>
-			<groupId>javax.servlet</groupId>
-			<artifactId>javax.servlet-api</artifactId>
-			<version>4.0.1</version>
-			<scope>provided</scope>
-		</dependency>
+		<dependency>
+			<groupId>jakarta.servlet</groupId>
+			<artifactId>jakarta.servlet-api</artifactId>
+			<scope>provided</scope>
+		</dependency>

,


71-73: Use BOM-managed version properties for protoc and grpc-java plugin.

Hardcoding 4.33.2 for protoc and 1.78.0 for grpc-java creates version drift risk when the BOM is updated. Reference the existing protobuf-java version from the BOM (line 827 declares 4.33.2) and add/use a grpc.version property.

♻️ Proposed fix
             <configuration>
-                <protocArtifact>com.google.protobuf:protoc:4.33.2:exe:${os.detected.classifier}</protocArtifact>
+                <protocArtifact>com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}</protocArtifact>
                 <pluginId>grpc-java</pluginId>
-                <pluginArtifact>io.grpc:protoc-gen-grpc-java:1.78.0:exe:${os.detected.classifier}</pluginArtifact>
+                <pluginArtifact>io.grpc:protoc-gen-grpc-java:${grpc.version}:exe:${os.detected.classifier}</pluginArtifact>
             </configuration>

,

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/openid/OpenIdProtectionService.java (1)

93-93: Inconsistent case-sensitivity in Bearer prefix removal.

The regex "Bearer\\s+" is case-sensitive, while other extractors in the codebase (e.g., AuthorizationProcessingFilter.extractBearerToken()) use case-insensitive "(?i)Bearer\\s+". This could cause failures when clients send lowercase bearer.

Proposed fix
-            bearerToken = bearerToken.replaceFirst("Bearer\\s+","");
+            bearerToken = bearerToken.replaceFirst("(?i)Bearer\\s+","");
jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/CedarlingProtectionService.java (2)

82-83: Security: Avoid logging bearer tokens and fix case-insensitive prefix removal.

  1. Line 83 logs the full bearer token at DEBUG level, risking credential leakage in logs.
  2. Line 82 uses case-sensitive regex "Bearer\\s+", inconsistent with other extractors that use "(?i)Bearer\\s+".
Proposed fix
-            bearerToken = bearerToken.replaceFirst("Bearer\\s+","");
-            log.debug("Validating token {}", bearerToken);
+            bearerToken = bearerToken.replaceFirst("(?i)Bearer\\s+","");
+            log.debug("Validating bearer token");

87-89: Use isEmpty() instead of size() == 0.

Prefer idiomatic collection emptiness check for readability.

Proposed fix
-            if (requestedPermissions.size() == 0) {
+            if (requestedPermissions.isEmpty()) {
jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/security/GrpcAuthorizationInterceptor.java (1)

94-103: Add null guard: deny access when ResourceInfo is not found.

extractResourceInfo() can return null when the gRPC method name format is invalid (line 174) or when no @ProtectedApi mapping exists (line 186). The current code proceeds to call processAuthorization(token, null), which may cause NPE or silent authorization bypass depending on the protection service implementation.

Fail-closed by rejecting requests when resource metadata cannot be resolved.

Suggested fix
         ResourceInfo resourceInfo = extractResourceInfo(methodName);
+        if (resourceInfo == null) {
+            log.warn("No ResourceInfo mapping for gRPC method: {}", methodName);
+            
+            // Audit the denied access
+            String clientIp = ServerUtil.getGrpcClientIpAddress(call, headers);
+            AuditLogEntry auditLogEntry = new AuditLogEntry(clientIp, AuditActionType.GRPC_AUTHZ_FILTER);
+            applicationAuditLogger.log(auditLogEntry, false);
+            
+            call.close(Status.PERMISSION_DENIED.withDescription("Authorization error: unknown method"), new Metadata());
+            return new ServerCall.Listener<ReqT>() {};
+        }
         log.debug("gRPC call requires access to: {}", resourceInfo);
jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/servlet/GrpcAuditServlet.java (3)

71-74: Fix typo in initialization log message.

"initializion" should be "initialization".

✏️ Proposed fix
-    	log.info("gRPC adapter initializion");
+    	log.info("gRPC adapter initialization");

156-163: Use Locale.ROOT for case-insensitive header name normalization.

The default String#toLowerCase() is locale-sensitive. Turkish locale transforms 'I' to 'ı', corrupting ASCII header names. Use Locale.ROOT for protocol identifiers.

Proposed fix
+import java.util.Locale;
-				headers.put(name.toLowerCase(), values);
+				headers.put(name.toLowerCase(Locale.ROOT), values);

Also update header lookups at lines 222 and 228:

-			List<String> values = headers.get(name.toLowerCase());
+			List<String> values = headers.get(name.toLowerCase(Locale.ROOT));

165-178: Normalize stripped URI to keep a leading "/".

Edge cases (e.g., contextPath equals /) can produce an empty or slash-less path after stripping, breaking gRPC method resolution.

🛠️ Proposed fix
 			if (contextPath != null && !contextPath.isEmpty() && originalURI.startsWith(contextPath)) {
-				this.requestURI = originalURI.substring(contextPath.length());
-				this.servletPath = this.requestURI;
+				String stripped = originalURI.substring(contextPath.length());
+				if (stripped.isEmpty()) {
+					stripped = "/";
+				} else if (!stripped.startsWith("/")) {
+					stripped = "/" + stripped;
+				}
+				this.requestURI = stripped;
+				this.servletPath = stripped;
 			} else {
jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/server/GrpcServerStarter.java (1)

68-103: useTls flag is ignored when deciding TLS vs plain.
If useTls=true but serverMode is left at its default (e.g., BRIDGE/PLAIN_SERVER), the server still starts without TLS. This makes the useTls field misleading.

🛠️ Suggested fix (derive an effective mode)
-    GrpcConfiguration grpcConfiguration = appConfiguration.getGrpcConfiguration();
-    if (grpcConfiguration == null || grpcConfiguration.getServerMode() == null ||
-            !(GrpcServerMode.PLAIN_SERVER == grpcConfiguration.getServerMode() || GrpcServerMode.TLS_SERVER == grpcConfiguration.getServerMode())) {
+    GrpcConfiguration grpcConfiguration = appConfiguration.getGrpcConfiguration();
+    if (grpcConfiguration == null || grpcConfiguration.getServerMode() == null) {
         log.info("gRPC inproc server was disabled in configuration");
         return;
     }
 
-    if (GrpcServerMode.TLS_SERVER == grpcConfiguration.getServerMode()) {
+    GrpcServerMode effectiveMode = grpcConfiguration.getServerMode();
+    if (grpcConfiguration.isUseTls() && effectiveMode == GrpcServerMode.PLAIN_SERVER) {
+        log.warn("useTls=true overrides serverMode=PLAIN_SERVER; starting TLS server");
+        effectiveMode = GrpcServerMode.TLS_SERVER;
+    }
+
+    if (!(GrpcServerMode.PLAIN_SERVER == effectiveMode || GrpcServerMode.TLS_SERVER == effectiveMode)) {
+        log.info("gRPC inproc server was disabled in configuration");
+        return;
+    }
+
+    if (GrpcServerMode.TLS_SERVER == effectiveMode) {
         // TLS path
         ...
-    } else {
+    } else {
         // Plain path
         ...
     }

yurem added 2 commits January 22, 2026 18:25
Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In
`@jans-lock/lock-server/server/src/test/manual/automatic_audit_enpoints_tests.sh`:
- Around line 11-15: Replace the hardcoded placeholders (HOST, GRPC_ADDR,
CLIENT_ID, CLIENT_SECRET, INVALID_TOKEN) with environment-driven values: read
each from process/env (e.g., HOST=${HOST:-} style) and enforce required checks
so the script exits with a clear error if any required variable (CLIENT_ID,
CLIENT_SECRET, HOST) is missing; allow an optional override for INVALID_TOKEN
and GRPC_ADDR, and document sourcing a local .env or exporting env vars in the
script header so secrets are never committed.
- Around line 171-239: The failure handlers currently echo the raw command and
output (see functions check_http_success, check_http_should_fail,
check_grpc_success, check_grpc_should_fail), which can leak Authorization:
Bearer tokens; sanitize before printing by constructing a safe_cmd that pipes
the original cmd through a redaction filter (e.g., sed to replace
'Authorization: Bearer <token>' with 'Authorization: Bearer <redacted>' and/or
remove '--header "Authorization: Bearer ..."' portions) and use safe_cmd in all
"Command:" logs, and similarly run the printed output through a redaction step
(e.g., safe_output=$(echo "$output" | sed -E 's/(Authorization:
Bearer)[[:space:]]+[^[:space:]]+/\1 <redacted>/gi')) before echoing "Output:" so
no bearer tokens appear in logs; update all places that echo $cmd or echo
"$output" in those four functions to use the redacted variables.
- Around line 202-218: The two bare redirections in the helper functions (the
standalone “> res.txt” lines immediately before the if eval blocks in the
functions such as check_grpc_should_fail) should be replaced with an explicit
no-op command to silence ShellCheck SC2188; update those lines to use a no-op
like : > res.txt (or true > res.txt) so the intent to truncate the file is
explicit while keeping the rest of the function (the eval "$cmd" >res.txt 2>&1
logic and the function names check_grpc_should_fail / the corresponding
pass-check function) unchanged.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/security/GrpcAuthorizationInterceptor.java`:
- Around line 212-236: The inner class GrpcResourceInfo currently holds an
implicit reference to its enclosing GrpcAuthorizationInterceptor instance;
declare it as a static nested class (change "class GrpcResourceInfo" to "static
class GrpcResourceInfo") so it no longer captures the outer instance, keeping
constructors and methods (getResourceMethod, getResourceClass, toString)
unchanged; ensure imports and usages inside GrpcAuthorizationInterceptor still
compile after making GrpcResourceInfo static.
- Around line 169-201: extractResourceInfo currently only inspects
AuditRestWebService.class and is therefore not extensible; change it to accept
or obtain a collection of service classes (e.g., pass a List<Class<?>> or use a
ServiceLoader/registry) and iterate over those when calling
getProtectionApiMethod so new gRPC service classes can be discovered without
modifying this method. In getProtectionApiMethod, replace
Optional.ofNullable(grpcResourceInfo) with Optional.of(grpcResourceInfo) because
grpcResourceInfo is never null at that point; keep referencing the methods
extractResourceInfo, getProtectionApiMethod, AuditRestWebService, and
GrpcResourceInfo to locate the changes.
- Around line 123-131: The catch block in GrpcAuthorizationInterceptor that logs
and closes the call on exceptions should also record an audit entry like the
existing authorization-failure path: call the same audit logging routine used
for authorization failures (use the same audit service/method invoked for
failures) with the methodName, the exception message and exception details
before or immediately after log.error and before call.close; ensure the audit
entry marks it as an exception/internal error and includes sufficient context
(methodName and exception) so it appears in the security audit trail.
♻️ Duplicate comments (3)
jans-lock/lock-server/service/src/main/java/io/jans/lock/util/ServerUtil.java (1)

146-173: Fix null pointer dereference and reverse IP resolution priority for security.

This code has two issues flagged in a previous review that remain unaddressed:

  1. NPE risk (line 161): inetAddr.getAddress() returns null for unresolved addresses, causing NullPointerException when getHostAddress() is called.

  2. Security concern (lines 148-153 vs 155-165): Client-controllable headers (X-Forwarded-For, X-Real-IP) are checked before the trusted socket address. Since this IP is used for authorization decisions in GrpcAuthorizationInterceptor, header spoofing is a security risk unless behind a trusted proxy. Reverse the priority to prefer socket address.

🔧 Proposed fix for null-safety and priority order
     public static String getGrpcClientIpAddress(io.grpc.ServerCall<?, ?> call, Metadata headers) {
         try {
-            // Method 1: Try to get from metadata headers
-            String ipFromHeaders = getIpFromHeaders(headers);
-            if (ipFromHeaders != null) {
-                log.debug("Client IP from headers: {}", ipFromHeaders);
-                return ipFromHeaders;
-            }
-
-            // Method 2: Get from ServerCall attributes
+            // Method 1: Get from ServerCall attributes (trusted source)
             if (call != null) {
                 io.grpc.Attributes attributes = call.getAttributes();
                 java.net.SocketAddress remoteAddr = attributes.get(io.grpc.Grpc.TRANSPORT_ATTR_REMOTE_ADDR);
                 if (remoteAddr instanceof java.net.InetSocketAddress) {
                     java.net.InetSocketAddress inetAddr = (java.net.InetSocketAddress) remoteAddr;
-                    String ip = inetAddr.getAddress().getHostAddress();
+                    java.net.InetAddress addr = inetAddr.getAddress();
+                    String ip = (addr != null) ? addr.getHostAddress() : inetAddr.getHostString();
                     log.debug("Client IP from ServerCall attributes: {}", ip);
                     return ip;
                 }
             }

+            // Method 2: Fall back to headers (only if behind trusted proxy)
+            String ipFromHeaders = getIpFromHeaders(headers);
+            if (ipFromHeaders != null) {
+                log.debug("Client IP from headers: {}", ipFromHeaders);
+                return ipFromHeaders;
+            }
+
             log.warn("Could not determine client IP address");
             return UNKNOWN;
         } catch (Exception e) {
jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/security/GrpcAuthorizationInterceptor.java (2)

65-66: Remove unused constant AUTHORIZATION_METADATA_KEY.

This constant is declared but never referenced. The code uses HeaderUtils.findAndExtractBearerToken(headers) at line 103 instead.

♻️ Proposed fix
-    private static final Metadata.Key<String> AUTHORIZATION_METADATA_KEY = 
-            Metadata.Key.of("Authorization", Metadata.ASCII_STRING_MARSHALLER);
-    
     `@Override`
     public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(

94-103: Add null guard: deny access when ResourceInfo cannot be resolved.

extractResourceInfo() returns null when the gRPC method name format is invalid (line 174) or when no @ProtectedApi mapping exists (line 186). Passing null to processAuthorization() will cause NullPointerException in both OpenIdProtectionService and CedarlingProtectionService when they call resourceInfo.getResourceMethod().

Fail-closed by rejecting requests when resource metadata cannot be resolved.

🔧 Proposed fix
         try {
             // Extract method-specific ResourceInfo
         	ResourceInfo resourceInfo = extractResourceInfo(methodName);
+            if (resourceInfo == null) {
+                log.warn("No ResourceInfo mapping for gRPC method: {}", methodName);
+                
+                String clientIp = ServerUtil.getGrpcClientIpAddress(call, headers);
+                AuditLogEntry auditLogEntry = new AuditLogEntry(clientIp, AuditActionType.GRPC_AUTHZ_FILTER);
+                applicationAuditLogger.log(auditLogEntry, false);
+                
+                call.close(Status.PERMISSION_DENIED.withDescription("Authorization error: unknown method"), new Metadata());
+                return new ServerCall.Listener<ReqT>() {};
+            }
             log.debug("gRPC call requires access to: {}", resourceInfo);

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

Quality Gate Passed Quality Gate passed for 'Jans authentication server parent'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In
`@jans-lock/lock-server/server/src/test/manual/automatic_audit_enpoints_tests.sh`:
- Around line 199-239: The helper functions check_grpc_success and
check_grpc_should_fail write to a hardcoded res.txt which is never removed;
create a temporary file at script start (e.g., RESFILE=$(mktemp)) and install a
trap to rm -f "$RESFILE" on EXIT (and ERR if desired), then update both
check_grpc_success and check_grpc_should_fail to redirect to "$RESFILE" instead
of res.txt so the file is unique and automatically cleaned up when the script
exits.
- Around line 1-16: Add explicit prerequisite checks immediately after set -euo
pipefail: verify binaries curl, jq, and grpcurl exist (use command -v or which
and exit non-zero with a clear message if missing) and verify the audit.proto
file exists (test -f and exit with a clear error). Place these checks near the
top of the script (after set -euo pipefail) so the script fails fast before
using HOST, GRPC_ADDR, CLIENT_ID, CLIENT_SECRET or INVALID_TOKEN; ensure each
check prints which prerequisite is missing and exits non-zero.

In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/security/GrpcAuthorizationInterceptor.java`:
- Around line 205-207: Rename the misspelled helper method optAnnnotation to
optAnnotation (change method signature: private static <T extends Annotation>
Optional<T> optAnnotation(AnnotatedElement elem, Class<T> cls)) and update all
call sites, notably the call inside getProtectedApiAnnotation, to use
optAnnotation; preserve the method's generics, return type, parameters and
static/private modifiers so behavior remains unchanged.
♻️ Duplicate comments (7)
jans-lock/lock-server/server/src/test/manual/automatic_audit_enpoints_tests.sh (3)

202-202: Fix bare redirections flagged by Shellcheck (SC2188).

Lines 202 and 217 contain bare redirections without a command. Use a no-op to make intent explicit.

🧹 Proposed fix
-  > res.txt
+  : > res.txt

Also applies to: 217-217


171-239: Redact bearer tokens before printing failed commands.

On failures, the script prints full commands including Authorization: Bearer …, which leaks credentials to logs/terminals.


11-15: Externalize host and credentials to avoid accidental secret commits.

Hardcoded placeholders invite local edits that can leak into commits. Prefer env overrides with required checks.

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/security/GrpcAuthorizationInterceptor.java (4)

91-100: Add null guard: deny access when ResourceInfo is not found.

extractResourceInfo() returns null when the gRPC method name format is invalid (line 171) or when no @ProtectedApi mapping exists (line 183). The code proceeds to call processAuthorization(token, null) at line 100, which will cause null reference errors or may silently allow unauthorized access.

Fail-closed by rejecting requests when resource metadata cannot be resolved.

Proposed fix
         ResourceInfo resourceInfo = extractResourceInfo(methodName);
+        if (resourceInfo == null) {
+            log.warn("No ResourceInfo mapping for gRPC method: {}", methodName);
+            
+            String clientIp = ServerUtil.getGrpcClientIpAddress(call, headers);
+            AuditLogEntry auditLogEntry = new AuditLogEntry(clientIp, AuditActionType.GRPC_AUTHZ_FILTER);
+            applicationAuditLogger.log(auditLogEntry, false);
+            
+            call.close(Status.PERMISSION_DENIED.withDescription("Unknown method"), new Metadata());
+            return new ServerCall.Listener<ReqT>() {};
+        }
         log.debug("gRPC call requires access to: {}", resourceInfo);

120-128: Add audit logging for exception cases.

Authorization failures at line 105 are audited, but exceptions caught here are not. For a complete security audit trail, exception cases should also be logged to maintain parity with the failure path.

Proposed fix
         } catch (Exception e) {
             log.error("Error during gRPC authorization for '{}'", methodName, e);
             
+            // Audit the failed authorization attempt
+            String clientIp = ServerUtil.getGrpcClientIpAddress(call, headers);
+            AuditLogEntry auditLogEntry = new AuditLogEntry(clientIp, AuditActionType.GRPC_AUTHZ_FILTER);
+            applicationAuditLogger.log(auditLogEntry, false);
+            
             call.close(Status.INTERNAL
                     .withDescription("Authorization error: " + e.getMessage())
                     .withCause(e), new Metadata());

166-184: Consider making service class discovery extensible.

extractResourceInfo only searches AuditRestWebService.class (line 177). If additional gRPC services are added later, this method requires modification. Consider accepting a list of service classes or using a registry pattern.


186-198: Use Optional.of() instead of Optional.ofNullable().

At line 192, grpcResourceInfo is constructed immediately before and is never null at that point. Using Optional.of() is more precise and communicates intent better.

Proposed fix
        if (protectedApi.isPresent()) {
       		if (grpcMethodName.equals(protectedApi.get().grpcMethodName())) {
       			GrpcResourceInfo grpcResourceInfo = new GrpcResourceInfo(clazz, method);
-				return Optional.ofNullable(grpcResourceInfo);
+				return Optional.of(grpcResourceInfo);
			}
        }

Comment on lines +1 to +16
#!/bin/bash

# =============================================================================
# Test script for checking REST ↔ gRPC bridge + Audit API authorization
# Tests: log, health, telemetry (single + bulk) with write & read scopes
# Requirements: curl, jq, grpcurl
# =============================================================================

set -euo pipefail

HOST="https://server.jans.info"
GRPC_ADDR="server.jans.info:443"
CLIENT_ID="2200...."
CLIENT_SECRET="..."
INVALID_TOKEN="this-is-definitely-not-a-valid-token"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add prerequisite checks for required tools and files.

The script depends on curl, jq, grpcurl, and audit.proto but doesn't verify their availability. This can cause confusing failures when prerequisites are missing.

🧹 Proposed fix — add checks after `set -euo pipefail`
 set -euo pipefail
+
+for cmd in curl jq grpcurl; do
+  command -v "$cmd" >/dev/null 2>&1 || { echo "Error: $cmd is required but not installed."; exit 1; }
+done
+
+[[ -f "audit.proto" ]] || { echo "Error: audit.proto not found in current directory."; exit 1; }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#!/bin/bash
# =============================================================================
# Test script for checking REST ↔ gRPC bridge + Audit API authorization
# Tests: log, health, telemetry (single + bulk) with write & read scopes
# Requirements: curl, jq, grpcurl
# =============================================================================
set -euo pipefail
HOST="https://server.jans.info"
GRPC_ADDR="server.jans.info:443"
CLIENT_ID="2200...."
CLIENT_SECRET="..."
INVALID_TOKEN="this-is-definitely-not-a-valid-token"
#!/bin/bash
# =============================================================================
# Test script for checking REST ↔ gRPC bridge + Audit API authorization
# Tests: log, health, telemetry (single + bulk) with write & read scopes
# Requirements: curl, jq, grpcurl
# =============================================================================
set -euo pipefail
for cmd in curl jq grpcurl; do
command -v "$cmd" >/dev/null 2>&1 || { echo "Error: $cmd is required but not installed."; exit 1; }
done
[[ -f "audit.proto" ]] || { echo "Error: audit.proto not found in current directory."; exit 1; }
HOST="https://server.jans.info"
GRPC_ADDR="server.jans.info:443"
CLIENT_ID="2200...."
CLIENT_SECRET="..."
INVALID_TOKEN="this-is-definitely-not-a-valid-token"
🤖 Prompt for AI Agents
In
`@jans-lock/lock-server/server/src/test/manual/automatic_audit_enpoints_tests.sh`
around lines 1 - 16, Add explicit prerequisite checks immediately after set -euo
pipefail: verify binaries curl, jq, and grpcurl exist (use command -v or which
and exit non-zero with a clear message if missing) and verify the audit.proto
file exists (test -f and exit with a clear error). Place these checks near the
top of the script (after set -euo pipefail) so the script fails fast before
using HOST, GRPC_ADDR, CLIENT_ID, CLIENT_SECRET or INVALID_TOKEN; ensure each
check prints which prerequisite is missing and exits non-zero.

Comment on lines +199 to +239
check_grpc_success() {
local cmd="$1"
local desc="$2"
> res.txt
if eval "$cmd" >res.txt 2>&1; then
echo "$desc"
else
echo "$desc"
echo "Command: $cmd"
echo "Output:"
cat res.txt
return 1
fi
}

check_grpc_should_fail() {
local cmd="$1"
local desc="$2"
> res.txt
if eval "$cmd" >res.txt 2>&1; then
echo "❌ UNEXPECTED success → $desc"
echo "Command: $cmd"
echo "Output:"
cat res.txt
return 1
else
local output
output=$(cat res.txt)
if echo "$output" | grep -q -i "PermissionDenied" || echo "$output" | grep -q -i "Invalid token" || echo "$output" | grep -q -i "unauthenticated"; then
local code
code=$(echo "$output" | grep -i "Code:" | head -n 1 | sed -E 's/.*Code:[[:space:]]*([^[:space:],().]+).*/\1/i')
echo "✅ (expected failure) $desc with status: $code"
else
echo "❌ UNEXPECTED gRPC error → $desc"
echo "Command: $cmd"
echo "Output:"
echo "$output"
return 1
fi
fi
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add cleanup for temporary file res.txt.

The gRPC helper functions create res.txt but never remove it. This could leave sensitive response data behind after the script exits.

🧹 Proposed fix — add trap at script start
 set -euo pipefail
+
+cleanup() {
+  rm -f res.txt
+}
+trap cleanup EXIT
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 202-202: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).

(SC2188)


[warning] 217-217: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).

(SC2188)

🤖 Prompt for AI Agents
In
`@jans-lock/lock-server/server/src/test/manual/automatic_audit_enpoints_tests.sh`
around lines 199 - 239, The helper functions check_grpc_success and
check_grpc_should_fail write to a hardcoded res.txt which is never removed;
create a temporary file at script start (e.g., RESFILE=$(mktemp)) and install a
trap to rm -f "$RESFILE" on EXIT (and ERR if desired), then update both
check_grpc_success and check_grpc_should_fail to redirect to "$RESFILE" instead
of res.txt so the file is unique and automatically cleaned up when the script
exits.

Comment on lines +205 to +207
private static <T extends Annotation> Optional<T> optAnnnotation(AnnotatedElement elem, Class<T> cls) {
return Optional.ofNullable(elem.getAnnotation(cls));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Typo in method name: optAnnnotation has three n's.

The method name contains a typo—it should be optAnnotation (two n's).

Proposed fix
-    private static <T extends Annotation> Optional<T> optAnnnotation(AnnotatedElement elem, Class<T> cls) {
+    private static <T extends Annotation> Optional<T> optAnnotation(AnnotatedElement elem, Class<T> cls) {
         return Optional.ofNullable(elem.getAnnotation(cls));
     }

Also update the call site in getProtectedApiAnnotation:

 	private Optional<ProtectedApi> getProtectedApiAnnotation(AnnotatedElement elem) {
-		Optional<ProtectedApi> protectedApi = optAnnnotation(elem, ProtectedApi.class);
+		Optional<ProtectedApi> protectedApi = optAnnotation(elem, ProtectedApi.class);
 		return protectedApi;
 	}
🤖 Prompt for AI Agents
In
`@jans-lock/lock-server/service/src/main/java/io/jans/lock/service/grpc/security/GrpcAuthorizationInterceptor.java`
around lines 205 - 207, Rename the misspelled helper method optAnnnotation to
optAnnotation (change method signature: private static <T extends Annotation>
Optional<T> optAnnotation(AnnotatedElement elem, Class<T> cls)) and update all
call sites, notably the call inside getProtectedApiAnnotation, to use
optAnnotation; preserve the method's generics, return type, parameters and
static/private modifiers so behavior remains unchanged.

Signed-off-by: Yuriy <Yuriy.Movchan@gmail.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/CedarlingProtectionService.java (3)

61-69: Silent failure in init() leaves service in broken state.

If getOpenIdConfiguration() fails, oidcConfig remains null and the service will throw NullPointerException at line 99 on first request. Consider failing fast at startup or adding defensive null checks.

Proposed fix: fail fast at startup
     `@PostConstruct`
     private void init() {
-        try {
-            mapper = new ObjectMapper();
-            oidcConfig = getOpenIdConfiguration();
-        } catch (Exception e) {
-            log.error(e.getMessage(), e);
-        }
+        mapper = new ObjectMapper();
+        oidcConfig = getOpenIdConfiguration();
     }

Alternatively, add a null check in processAuthorization() and return INTERNAL_SERVER_ERROR if oidcConfig is null.


166-167: Edge case: Integer.MIN_VALUE negation produces negative result.

When hashCode() returns Integer.MIN_VALUE, negating it still yields a negative value due to two's complement overflow. If a positive ID is required, use a bitwise mask.

Proposed fix
-		int id = requestedPermission.hashCode();
-		id = id > 0 ? id : -id;
+		int id = requestedPermission.hashCode() & Integer.MAX_VALUE;

117-119: Performance: JWKS fetched on every authorization request.

mapper.readValue(new URL(oidcConfig.getJwksUri()), Map.class) makes an HTTP call for each request. JWKS keys rotate infrequently; consider caching with a reasonable TTL (e.g., 5-10 minutes) or using a JWKS client with built-in caching.

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/openid/OpenIdProtectionService.java (1)

81-94: Guard against blank token after prefix stripping.

StringUtils.isNotEmpty allows whitespace-only values; after replaceFirst, inputs like "Bearer " become empty and flow into introspection, which can end up as a 500 instead of 401. Re-check after stripping and trim before continuing.

🛠️ Proposed fix
-            bearerToken = bearerToken.replaceFirst("Bearer\\s+","");
-            log.debug("Validating bearer token");
+            bearerToken = bearerToken.replaceFirst("^Bearer\\s+","").trim();
+            if (StringUtils.isBlank(bearerToken)) {
+                log.debug("Request is missing bearer token");
+                return simpleResponse(UNAUTHORIZED, "No bearer token found");
+            }
+            log.debug("Validating bearer token");
🤖 Fix all issues with AI agents
In
`@jans-core/service/src/main/java/io/jans/service/security/api/ProtectedApi.java`:
- Around line 20-23: Update the Javadoc on the grpcMethodName() element of the
ProtectedApi interface: remove the surrounding backticks so the Javadoc tag is a
proper `@return` tag (e.g. change "`@return` gRPC method name..." to "@return gRPC
method name mapped to this endpoint (empty if not applicable)"). Ensure the tag
appears on its own line in the Javadoc block above the String grpcMethodName()
default "" declaration.

In
`@jans-lock/lock-server/service/src/main/policy/audit/telemetry_bulk_write.json`:
- Around line 1-11: The Cedar policy files (e.g., the policy with
`@id`("lock_audit_telemetry_write") stored as telemetry_bulk_write.json) are using
Cedar syntax but have a .json extension, which will cause Biome JSON parsing
errors; fix this by either renaming these policy files to use a .cedar extension
(e.g., telemetry_bulk_write.cedar, health_write.cedar, get_policy.cedar) or add
a Biome override in biome.json that disables the linter for the policy directory
(targeting the policy path under lock-server/service/src/main/policy) so Biome
skips parsing these files as JSON.
♻️ Duplicate comments (5)
jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/inject/CedarlingPolicy.java (1)

2-4: License header still uses non‑standard Apache reference and year span.

Same issue previously flagged; please apply the standard “Apache License, Version 2.0” wording and adjust the copyright years to reflect the 2022–2025 span.

jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/CedarlingProtectionService.java (2)

82-83: Security: Avoid logging bearer tokens and fix case-insensitive prefix removal.

The issues identified in the previous review are still present:

  1. Line 83 logs the full bearer token at DEBUG level, risking credential leakage.
  2. Line 82 uses case-sensitive regex "Bearer\\s+", while CedarlingAuthorizationProcessingFilter.extractBearerToken() uses case-insensitive "(?i)Bearer\\s+". This inconsistency can cause authorization failures.
Proposed fix
-            bearerToken = bearerToken.replaceFirst("Bearer\\s+","");
-            log.debug("Validating token {}", bearerToken);
+            bearerToken = bearerToken.replaceFirst("(?i)Bearer\\s+","");
+            log.debug("Validating bearer token");

87-89: Use isEmpty() instead of size() == 0.

Prefer idiomatic collection emptiness check as noted in the previous review.

Proposed fix
-            if (requestedPermissions.size() == 0) {
+            if (requestedPermissions.isEmpty()) {
jans-lock/lock-server/service/src/main/policy/audit/log_bulk_write.json (1)

1-11: Same concerns as telemetry bulk policy.
Scope type safety and Biome/extension handling apply equally here.

jans-lock/lock-server/service/src/main/policy/audit/health_bulk_write.json (1)

1-11: Same concerns as telemetry bulk policy.
Scope type safety and Biome/extension handling apply equally here.

Comment on lines +20 to +23
/**
* `@return` gRPC method name mapped to this endpoint (empty if not applicable)
*/
String grpcMethodName() default "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Fix Javadoc syntax: remove backticks around @return.

The backticks around @return on line 21 cause it to be rendered as literal text rather than parsed as a proper Javadoc tag.

Proposed fix
 	/**
-	 * `@return` gRPC method name mapped to this endpoint (empty if not applicable)
+	 * `@return` gRPC method name mapped to this endpoint (empty if not applicable)
 	 */
 	String grpcMethodName() default "";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* `@return` gRPC method name mapped to this endpoint (empty if not applicable)
*/
String grpcMethodName() default "";
/**
* `@return` gRPC method name mapped to this endpoint (empty if not applicable)
*/
String grpcMethodName() default "";
🤖 Prompt for AI Agents
In
`@jans-core/service/src/main/java/io/jans/service/security/api/ProtectedApi.java`
around lines 20 - 23, Update the Javadoc on the grpcMethodName() element of the
ProtectedApi interface: remove the surrounding backticks so the Javadoc tag is a
proper `@return` tag (e.g. change "`@return` gRPC method name..." to "@return gRPC
method name mapped to this endpoint (empty if not applicable)"). Ensure the tag
appears on its own line in the Javadoc block above the String grpcMethodName()
default "" declaration.

Comment on lines +1 to +11
@id("lock_audit_telemetry_write")
permit(
principal is Jans::Workload,
action in Jans::Action::"POST",
resource == Jans::HTTP_Request::"lock_audit_telemetry_write"
)
when {
principal has access_token.scope &&
principal.access_token.scope.contains("https://jans.io/oauth/lock/telemetry.write") &&
resource has url && resource.url has path && resource.url.path == "/audit/telemetry/bulk"
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check Biome configuration for file ignores/overrides.
fd -t f 'biome.*'
rg -n 'ignore|overrides|files|include|exclude' -g 'biome.*'

Repository: JanssenProject/jans

Length of output: 373


🏁 Script executed:

cat -n biome.json

Repository: JanssenProject/jans

Length of output: 2356


🏁 Script executed:

# Verify the Cedar policy file exists and check its content
cat -n jans-lock/lock-server/service/src/main/policy/audit/telemetry_bulk_write.json | head -15

Repository: JanssenProject/jans

Length of output: 540


🏁 Script executed:

# Check if there's a CI configuration that runs Biome
fd -t f '\.(yml|yaml|json)' -path '*/.github/workflows/*' -o -path '*/\.gitlab-ci\.yml' -o -path '*/\.circleci/*'
# Also check for any CI lint/check scripts
fd -t f 'Makefile|build\.sh|lint\.' | head -20

Repository: JanssenProject/jans

Length of output: 938


🏁 Script executed:

# Check GitHub workflows for Biome linting
fd -type f '\.yml' .github/workflows/ 2>/dev/null | head -20
# Also check for any scripts that might run Biome
rg -l 'biome' --type sh --type yml --type yaml 2>/dev/null | head -20

Repository: JanssenProject/jans

Length of output: 45


🏁 Script executed:

# Check if .github/workflows directory exists and list files
ls -la .github/workflows/ 2>/dev/null | head -20
# Search for any reference to Biome in the entire repository
rg 'biome' -i --type json --type yml --type yaml --type sh 2>/dev/null

Repository: JanssenProject/jans

Length of output: 1342


🏁 Script executed:

# Search all workflow files for Biome references
rg 'biome' .github/workflows/ -i
# Also check if the policy directory has any ignore patterns in place
ls -la jans-lock/lock-server/service/src/main/policy/ 2>/dev/null

Repository: JanssenProject/jans

Length of output: 333


🏁 Script executed:

# Check if there are any .cedar files in the policy directory
find jans-lock/lock-server/service/src/main/policy/ -type f
# Check if biome actually attempts to parse these files by looking for any biome ignore patterns
rg 'ignore|include|exclude' biome.json -A 2 -B 2

Repository: JanssenProject/jans

Length of output: 766


🏁 Script executed:

# Check for .biomeignore or any other Biome ignore files
find . -name '.biomeignore' -o -name 'biome-ignore*' 2>/dev/null
# Check the build-test.yml to see if it runs Biome
cat .github/workflows/build-test.yml | grep -A 10 -B 5 'biome\|lint'

Repository: JanssenProject/jans

Length of output: 45


🏁 Script executed:

# Let's verify that these Cedar files truly contain invalid JSON syntax
python3 << 'EOF'
import json
import os

policy_files = [
    'jans-lock/lock-server/service/src/main/policy/audit/telemetry_bulk_write.json',
    'jans-lock/lock-server/service/src/main/policy/audit/health_write.json',
    'jans-lock/lock-server/service/src/main/policy/policy/get_policy.json'
]

for policy_file in policy_files:
    try:
        with open(policy_file, 'r') as f:
            json.load(f)
        print(f"✓ {policy_file}: Valid JSON")
    except json.JSONDecodeError as e:
        print(f"✗ {policy_file}: Invalid JSON - {e}")
    except FileNotFoundError:
        print(f"? {policy_file}: File not found")
EOF

Repository: JanssenProject/jans

Length of output: 149


🏁 Script executed:

# Check one specific Cedar file to see if it's valid JSON
head -1 jans-lock/lock-server/service/src/main/policy/audit/telemetry_bulk_write.json
# Try to parse with jq
jq empty jans-lock/lock-server/service/src/main/policy/audit/telemetry_bulk_write.json 2>&1 | head -5

Repository: JanssenProject/jans

Length of output: 153


Cedar policy files with .json extension will fail Biome JSON parsing.

The policy directory contains 10 Cedar policy files (e.g., telemetry_bulk_write.json, health_write.json, get_policy.json) with .json extensions but Cedar syntax (@id, permit, when blocks), not JSON. Biome's linter is enabled in biome.json with no ignore/exclude patterns for this directory. If Biome processes *.json files, it will fail to parse these Cedar policies.

Rename to .cedar extension or add an ignore pattern in biome.json:

{
  "overrides": [
    {
      "include": ["jans-lock/lock-server/service/src/main/policy/**"],
      "linter": { "enabled": false }
    }
  ]
}
🧰 Tools
🪛 Biome (2.1.2)

[error] 1-1: unexpected character @

(parse)


[error] 1-1: String values must be double quoted.

(parse)


[error] 1-1: unexpected character (

(parse)


[error] 1-1: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 1-1: unexpected character )

(parse)


[error] 1-2: String values must be double quoted.

(parse)


[error] 2-2: unexpected character (

(parse)


[error] 3-3: String values must be double quoted.

(parse)


[error] 3-3: String values must be double quoted.

(parse)


[error] 3-3: String values must be double quoted.

(parse)


[error] 3-4: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 4-4: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 4-5: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 5-5: unexpected character =

(parse)


[error] 5-5: unexpected character =

(parse)


[error] 5-5: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 5-6: unexpected character )

(parse)


[error] 6-7: String values must be double quoted.

(parse)


[error] 8-8: Property key must be double quoted

(parse)


[error] 8-8: expected : but instead found has

Remove has

(parse)


[error] 8-8: expected , but instead found access_token

Remove access_token

(parse)


[error] 8-8: unexpected character .

(parse)


[error] 8-8: expected , but instead found scope

Remove scope

(parse)


[error] 8-8: unexpected character &

(parse)


[error] 8-8: unexpected character &

(parse)


[error] 9-9: expected , but instead found principal

Remove principal

(parse)


[error] 9-9: unexpected character .

(parse)


[error] 9-9: expected , but instead found access_token

Remove access_token

(parse)


[error] 9-9: unexpected character .

(parse)


[error] 9-9: expected , but instead found scope

Remove scope

(parse)


[error] 9-9: unexpected character .

(parse)


[error] 9-9: expected , but instead found contains

Remove contains

(parse)


[error] 9-9: unexpected character (

(parse)


[error] 9-9: expected , but instead found "https://jans.io/oauth/lock/telemetry.write"

Remove "https://jans.io/oauth/lock/telemetry.write"

(parse)


[error] 9-9: unexpected character )

(parse)


[error] 9-9: unexpected character &

(parse)


[error] 9-9: unexpected character &

(parse)


[error] 10-10: expected , but instead found resource

Remove resource

(parse)


[error] 10-10: expected : but instead found has

Remove has

(parse)


[error] 10-10: expected , but instead found url

Remove url

(parse)


[error] 10-10: unexpected character &

(parse)


[error] 10-10: unexpected character &

(parse)


[error] 10-10: unexpected character .

(parse)


[error] 10-10: unexpected character &

(parse)


[error] 10-10: unexpected character &

(parse)


[error] 10-10: unexpected character .

(parse)


[error] 10-10: unexpected character .

(parse)


[error] 10-10: unexpected character =

(parse)


[error] 10-10: unexpected character =

(parse)


[error] 10-10: expected , but instead found "/audit/telemetry/bulk"

Remove "/audit/telemetry/bulk"

(parse)


[error] 10-11: expected : but instead found }

Remove }

(parse)


[error] 7-11: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)


[error] 11-11: unexpected character ;

(parse)

🤖 Prompt for AI Agents
In
`@jans-lock/lock-server/service/src/main/policy/audit/telemetry_bulk_write.json`
around lines 1 - 11, The Cedar policy files (e.g., the policy with
`@id`("lock_audit_telemetry_write") stored as telemetry_bulk_write.json) are using
Cedar syntax but have a .json extension, which will cause Biome JSON parsing
errors; fix this by either renaming these policy files to use a .cedar extension
(e.g., telemetry_bulk_write.cedar, health_write.cedar, get_policy.cedar) or add
a Biome override in biome.json that disables the linter for the policy directory
(targeting the policy path under lock-server/service/src/main/policy) so Biome
skips parsing these files as JSON.

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp-docs Touching folder /docs comp-jans-auth-server Component affected by issue or PR comp-jans-bom Component affected by issue or PR comp-jans-config-api Component affected by issue or PR comp-jans-core Component affected by issue or PR comp-jans-fido2 Component affected by issue or PR comp-jans-linux-setup Component affected by issue or PR comp-jans-lock comp-jans-scim Component affected by issue or PR kind-feature Issue or PR is a new feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(jans-lock): ad gPRC audit endpoints support

4 participants